sourcegraph: multi-tenant Zoekt #859
Conversation
- prefix flag instead of bool - padded ids - include tenant in id - add tenant tests
|
Fuzz test failed on commit b93cb3c. To troubleshoot locally, use the GitHub CLI to download the seed corpus with |
stefanhengl
changed the title
build/e2e_test.go
Outdated
| @@ -153,6 +158,88 @@ func TestBasic(t *testing.T) { | |||
| }) | |||
| } | |||
|
|
|||
| func TestBasicTenant(t *testing.T) { | |||
There was a problem hiding this comment.
minor: should this test live inside of the tenant package?
There was a problem hiding this comment.
That would create circular imports. Why do you want to move it? To not spill too much "tenant" into the codebase? We could put it in a separate package inside internal/tenant/
| @@ -592,6 +601,7 @@ func (s *Server) Index(args *indexArgs) (state indexState, err error) { | |||
| case build.IndexStateMeta: | |||
| log.Printf("updating index.meta %s", args.String()) | |||
|
|
|||
| // TODO(stefan) handle mergeMeta for tenant id. | |||
There was a problem hiding this comment.
:O also need exploding then to understand the new naming scheme?
There was a problem hiding this comment.
This here is a bit different. We should probably trigger a reindex if the tenant id changes, but I didn't want to include it now because that would trigger a reindex for everyone.
For explode, I updated the naming gated by tenant enforcement. I will file a follow-up to make that nicer. It's obviously too brittle.
Co-authored-by: Keegan Carruthers-Smith <[email protected]>
Co-authored-by: Keegan Carruthers-Smith <[email protected]>
This updates webserver and sourcegraph-indexserver to support multi-tenancy.
The change is behind an ENV feature-flag.
Key changes:
TenantIdAssumptions:
Test plan:
This is how the new Sourcegraph naming scheme looks like on disk:
